Initial support of asg and alb. #11

jsauter · 2018-09-18T16:14:52Z

Initial implementation of the alb and asg steps.

This pull request contains steps that will facilitate the deployment of an alb and asg. There are some assumptions around scaling groups and the creation of an alb at this point.

cumulus/components/userdata/linux.py

brettswift · 2018-09-18T17:01:14Z

cumulus/steps/dev_tools/pipeline.py

        generic_pipeline = codepipeline.Pipeline(
            "Pipeline",
-            # Name=chain_context.instance_name,
+            Name=chain_context.instance_name,


How come this is being added back in ?

Conflict error, will fix.

brettswift · 2018-09-18T17:01:45Z

cumulus/steps/ec2/alb.py

+        alb_sg = ALB_SG_NAME % self.name
+
+        # TODO: this SG shouldn't be required.. test.
+        template.add_resource(ec2.SecurityGroup(


Should probably get to that eventually in the infra blueprint that this came from.

we're not using the one where it came from anymore.

I have removed this one, and I don't see any ramifications in the net result, so I will leave it out.

brettswift · 2018-09-18T17:04:17Z

cumulus/steps/ec2/alb.py

+
+    def create_default_target_group(self, template):
+        template.add_resource(alb.TargetGroup(
+            TARGET_GROUP_DEFAULT % self.name,


If we run two instances of this, ie a bswift and a sauter instance separately, this looks like we'll get name collisions. Prefix support in cumulus is probably a bigger discussion.

brettswift · 2018-09-18T17:07:20Z

cumulus/steps/ec2/dns.py

+                        EvaluateTargetHealth=False,
+                    ),
+                    Name=Join("", [
+                        Ref("stackname"),


Not sure if this was brought over from the builder project but we have a concept of self.name, chain_context.instance_name, and now Ref("stackname").

Can we streamline these ? Is stackname really required?

Probably not required, can use the other options if they are providing a stackname.

brettswift · 2018-09-18T17:10:11Z

cumulus/steps/ec2/ingress_rule.py

+            FromPort=self.port_to_open,
+            ToPort=self.port_to_open,
+            CidrIp=self.cidr,
+            GroupId=Ref(chain_context.metadata[META_SECURITY_GROUP_NAME])


This is a slight departure from a convention the pipeline introduced.. it's not a big deal but might be good to streamline as much as possible.
Instead of popping the name on the metadata, pipeline put the Ref object on it. Which makes the most sense? Should the pipeline change or should this?

I found putting it on the metadata was pretty intuitive!

so did I. The question was should we put a string or a ref. If we stick with the precedence in master we'd put a Ref object on the metadata.

Either works. I don't see either case being more or less easy to work with.

I will make this one use a ref on the metadata.

brettswift · 2018-09-18T17:11:30Z

cumulus/steps/ec2/launch_config.py

+        self.meta_data = meta_data
+        self.instance_profile_name = instance_profile_name
+        self.vpc_id = vpc_id
+


It seems like the following could be derived, and at least made optional here, to simplify the consumer requirements.

asg_name

launch_config_name

instance_profile_name

brettswift · 2018-09-18T17:14:04Z

cumulus/steps/ec2/launch_config.py

+        asg_sg_list = [self._get_launch_configuration_security_groups(chain_context.metadata[META_SECURITY_GROUP_NAME])]
+
+        parameters = {
+            'ImageId': FindInMap('AmiMap',


ugh, so like stacker and parameters, stacker has an opinion on mappings.

These mappings are required to exist in stacker.yaml, for the sample code here. This poses another problem if cumulus is going to be non-stacker specific..

I think the best way to do this would be to supply a way to generate mappings in the way cumulus wants. Something for a separate story/feature.

issue #14 created.

for this PR we should probably just document an example mapping section in the class docs of the class.

brettswift · 2018-09-18T17:15:25Z

cumulus/steps/ec2/listener_rule.py

+                Values=[
+                    Ref("stackname"),
+                    "-",
+                    Ref("namespace"),


namespace, and stackname again. Are these bleeding in from stacker? Namespace is a stacker concept.. although the code that this was brought in from.. (the builder) didn't have a working host_pattern. So maybe this could be tackled separately.. as I'm guessing this isn't tested in this PR.

A fix / documents here are still pending.

brettswift · 2018-09-18T17:17:36Z

cumulus/steps/ec2/port.py

+from cumulus.chain import step
+
+
+class Port(step.Step):


The name here is too generic. This is specifically a class for ASG--> ALB traffic.

I think it's ok to nuke this class and have this done directly where the target group is added. Likely in the consumer code there are ports being passed into a few places.

brettswift · 2018-09-18T17:18:29Z

cumulus/steps/ec2/role.py

+from cumulus.util.tropo import TemplateQuery
+
+
+class Role(step.Step):


This role is specific to an instance profile, so we might want to name it specifically that.

There will be many other things that want to call themselves "Role_xxx".

brettswift · 2018-09-18T17:19:44Z

cumulus/steps/ec2/target_group.py

+
+        # todo: why is this not allowing a reference?
+
+        name = 'CumulusTargetGroup'


This will cause collisions on multiple instances.

brettswift · 2018-09-18T17:20:13Z

cumulus/steps/ec2/scaling_group.py

+from cumulus.steps.ec2 import META_SECURITY_GROUP_NAME, META_TARGET_GROUP_NAME
+
+
+class LaunchType:


duplication, also exists in cumulus/types.py

remove one of them

brettswift · 2018-09-18T17:22:20Z

tests/stacker_test/blueprints/scaling_group_simple.py

+        the_chain.add(ingress_rule.IngressRule(
+            name="SgCidrAccess",
+            port_to_open="22",
+            asg_sg_name="sg1" + self.name,


as above, this is the client code that gets really confusing, having to pass around asg_sg_name in a bunch of places. When it gets derived and pulled into one of the steps we won't have to worry about it here.

I think the user needs to know too much about steps and sequencing

cumulus/components/userdata/windows.py

Add flake8 to the ci build

Fix flake8 errors from PR #11

jsauter force-pushed the feature/scaling_group_steps branch from d768497 to e3a12ab Compare September 18, 2018 16:53

brettswift reviewed Sep 18, 2018

View reviewed changes

cumulus/components/userdata/linux.py Show resolved Hide resolved

brettswift reviewed Sep 18, 2018

View reviewed changes

jsauter force-pushed the feature/scaling_group_steps branch from 217ab94 to 46186d0 Compare September 18, 2018 20:41

brettswift mentioned this pull request Sep 20, 2018

Mapping support #14

Open

john-shaskin reviewed Sep 20, 2018

View reviewed changes

cumulus/components/userdata/windows.py Show resolved Hide resolved

Initial support of asg and alb.

03b9a82

jsauter force-pushed the feature/scaling_group_steps branch from 46186d0 to 03b9a82 Compare September 21, 2018 21:09

brettswift merged commit 460662e into brettswift:master Sep 25, 2018

brettswift pushed a commit that referenced this pull request Sep 25, 2018

Fix flake8 errors from PR #11

cf82835

brettswift pushed a commit that referenced this pull request Sep 25, 2018

Fix flake8 errors from PR #11

1f2ffd8

Add flake8 to the ci build

brettswift added a commit that referenced this pull request Sep 25, 2018

Merge pull request #18 from brettswift/fix/flake8

fe93b5b

Fix flake8 errors from PR #11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial support of asg and alb. #11

Initial support of asg and alb. #11

jsauter commented Sep 18, 2018 •

edited

Loading

brettswift Sep 18, 2018

jsauter Sep 18, 2018

brettswift Sep 18, 2018

jsauter Sep 18, 2018

brettswift Sep 20, 2018

jsauter Sep 21, 2018

brettswift Sep 18, 2018

brettswift Sep 18, 2018

jsauter Sep 18, 2018

brettswift Sep 18, 2018

jsauter Sep 18, 2018

brettswift Sep 20, 2018

jsauter Sep 20, 2018

jsauter Sep 20, 2018

brettswift Sep 18, 2018

brettswift Sep 18, 2018

brettswift Sep 20, 2018

brettswift Sep 20, 2018

brettswift Sep 18, 2018

brettswift Sep 20, 2018

brettswift Sep 18, 2018

brettswift Sep 20, 2018

brettswift Sep 18, 2018

brettswift Sep 20, 2018

brettswift Sep 18, 2018

brettswift Sep 20, 2018

brettswift Sep 18, 2018

brettswift Sep 20, 2018

brettswift Sep 18, 2018

		from cumulus.util.tropo import TemplateQuery


		class Role(step.Step):


		# todo: why is this not allowing a reference?

		name = 'CumulusTargetGroup'

		from cumulus.steps.ec2 import META_SECURITY_GROUP_NAME, META_TARGET_GROUP_NAME


		class LaunchType:

Initial support of asg and alb. #11

Initial support of asg and alb. #11

Conversation

jsauter commented Sep 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsauter commented Sep 18, 2018 •

edited

Loading